[Clang][CodeGen] Fix bad codegen when building Clang with latest MSVC #102681
Conversation
aganea
requested review from
compnerd,
davidchisnall,
MaskRay,
AaronBallman and
dwblaikie
llvmbot
added
clang
|
@llvm/pr-subscribers-clang-codegen @llvm/pr-subscribers-clang Author: Alexandre Ganea (aganea) ChangesBefore this PR, when using the latest MSVC This seems to have been introduced by c9e5af3 however further inspection shows that commit only triggers a bug in the MSVC compiler. It seems that the symptom is bad alignement generated in one of the load instructions: When
The code line on the right is translated to the assembly on the right. I'll fix a bug with Microsoft will cross post it here. Full diff: https://github.com/llvm/llvm-project/pull/102681.diff 1 Files Affected:
diff --git a/clang/lib/CodeGen/CGObjCGNU.cpp b/clang/lib/CodeGen/CGObjCGNU.cpp
index 948b10954ebbe..56a3ed1b87b35 100644
--- a/clang/lib/CodeGen/CGObjCGNU.cpp
+++ b/clang/lib/CodeGen/CGObjCGNU.cpp
@@ -2092,10 +2092,10 @@ class CGObjCGNUstep2 : public CGObjCGNUstep {
auto *classStart =
llvm::StructType::get(PtrTy, PtrTy, PtrTy, LongTy, LongTy);
auto &astContext = CGM.getContext();
- auto flags = Builder.CreateLoad(
- Address{Builder.CreateStructGEP(classStart, selfValue, 4), LongTy,
- CharUnits::fromQuantity(
- astContext.getTypeAlign(astContext.UnsignedLongTy))});
+ llvm::Value *Val = Builder.CreateStructGEP(classStart, selfValue, 4);
+ auto Align = CharUnits::fromQuantity(
+ astContext.getTypeAlign(astContext.UnsignedLongTy));
+ auto flags = Builder.CreateLoad(Address{Val, LongTy, Align});
auto isInitialized =
Builder.CreateAnd(flags, ClassFlags::ClassFlagInitialized);
llvm::BasicBlock *notInitializedBlock =
|
|
The new version is probably slightly more readable than mine, but I don’t really understand how it would affect codegen with UT C. No objections. |
I can add a comment to that effect. |
Before this PR, when using the latest MSVC
Microsoft (R) C/C++ Optimizing Compiler Version 19.40.33813 for x64one of the Clang unit test used to fail:CodeGenObjC/gnustep2-direct-method.m, see full failure log here.This seems to have been introduced by c9e5af3 however further inspection shows that commit only triggers a bug in the MSVC compiler.
It seems that the symptom is bad alignement generated in one of the load instructions:
When
Builder.CreateLoadis called here, somehow this call toAddr.getAlignment().getAsAlign()returns a bad alignement. The problem occurs at the highlighted line in the screenshot (sub bh,cl):The code line on the right is translated to the assembly on the left.
llvm::count_zeroreturns a proper value (as seen inrcx), howeversub bh, cluses a bad constant inbh(it is not 63 as expected). I think the optimizer meant to usedilnotbh. A few lines below it doesmov byte ptr [rsp + 40h], dil. If aftersubis executed I manually set 6 inrdi, as it should have been, the test passes.This happens only in optimized builds. I used
llvm\utils\release\build_llvm_release.bat --version 19.0.0 --x64 --skip-checkout --local-pythonto build a LLVM installer.Bug reported here: https://developercommunity.visualstudio.com/t/Bad-code-generation-when-building-LLVM-w/10719589?port=1025&fsid=e572244a-cde7-4d75-a73d-9b8cd94204dd